-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add noble as an Ubuntu distro of choice. #738
Conversation
create_jenkins_job.py
Outdated
@@ -120,7 +120,7 @@ def main(argv=None): | |||
'enable_coverage_default': 'false', | |||
'dont_notify_every_unstable_build': 'false', | |||
'build_timeout_mins': 0, | |||
'ubuntu_distro': 'jammy', | |||
'ubuntu_distro': 'noble', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to switch the default to noble
yet, so let's leave this as jammy
for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed back in 89f939d.
linux_docker_resources/Dockerfile
Outdated
@@ -94,7 +94,7 @@ RUN if test \( ${UBUNTU_DISTRO} = jammy -a ${ROS_DISTRO} != humble \); then apt- | |||
# Iron uses python3-lttng, but then starting after Iron we switch to liblttng-ctl-dev. | |||
RUN if test \( ${UBUNTU_DISTRO} = jammy -a ${ROS_DISTRO} = iron \); then apt-get update && apt-get install --no-install-recommends -y \ | |||
python3-lttng; fi | |||
RUN if test \( ${UBUNTU_DISTRO} = jammy -a ${ROS_DISTRO} = rolling \); then apt-get update && apt-get install --no-install-recommends -y \ | |||
RUN if test \( ${UBUNTU_DISTRO} = noble -a ${ROS_DISTRO} = rolling \); then apt-get update && apt-get install --no-install-recommends -y \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, we want to support Rolling on both jammy
and noble
for now. So please update the conditional so either will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 89f939d.
linux_docker_resources/Dockerfile
Outdated
@@ -94,7 +94,7 @@ RUN if test \( ${UBUNTU_DISTRO} = jammy -a ${ROS_DISTRO} != humble \); then apt- | |||
# Iron uses python3-lttng, but then starting after Iron we switch to liblttng-ctl-dev. | |||
RUN if test \( ${UBUNTU_DISTRO} = jammy -a ${ROS_DISTRO} = iron \); then apt-get update && apt-get install --no-install-recommends -y \ | |||
python3-lttng; fi | |||
RUN if test \( ${UBUNTU_DISTRO} = jammy -a ${ROS_DISTRO} = rolling \); then apt-get update && apt-get install --no-install-recommends -y \ | |||
RUN if test \(\( ${UBUNTU_DISTRO} = jammy -o ${UBUNTU_DISTRO} = noble \) -a ${ROS_DISTRO} = rolling \); then apt-get update && apt-get install --no-install-recommends -y \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After looking at this and thinking about it again, I think I steered you wrong. In particular, we don't need the UBUNTU_DISTRO
check at all, we can really just get by with the ROS_DISTRO
check. I'm going to push a new commit here that does that, along with simplifies a few of the above lines.
28bc334
to
e3c4636
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me with green CI.
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
It is enough to just have the ROS_DISTRO checks here. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
e3c4636
to
420e1ef
Compare
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
test_ci_linux: test_ci_linux-aarch64: test_packaging_linux: test_packaging_linux-aarch64: Edit: I managed to manually update the subtree and create a manual squash merge commit that is usable by the |
git-subtree-dir: ros2_batch_job/vendor/osrf_pycommon git-subtree-split: f65f8594161dcff276813d59d3c1145ce767bb75 Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
cdf08a1
to
b1a438c
Compare
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
I realized if we |
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
Overall, this looks great to me. I'm going to suggest running a couple more pieces of CI on this, because of the change to ros2_batch_job/util.py. In particular, we are still using old versions of Python on both Rolling Windows (Python 3.8.3), and on Humble RHEL-8 (3.6). So it is possible that we will run into problems on those platforms. So please run CI with this branch on both of those. Once we confirm that those are good, I'm happy to approve and merge this PR. Thanks! |
Windows Debug is failing, but that is a completely different issue unrelated to this PR. This otherwise looks good to me. I'm going to approve it, merge it in, and deploy it so we get a full day of running it to be sure it still works. |
And this is now deployed! |
Starting this PR to gather feedback.
I believe with should hold the merge until the bootstrap repository step is done.
Note that: